@microsoft-github-policy-service agree company="pitt&sherry"
@microsoft-github-policy-service agree company="Microsoft"

This would be very handy to have merged and released

@dixonte One thing though, in InProc we used to deal with HttpResponseMessage as return type, I see you went for HttpResponseData. Any particular reason for that?

@MarioMajcicaAtABNAMRO That's what the main library was using for isolated worker.

@MarioMajcicaAtABNAMRO That's what the main library was using for isolated worker.

For the InProcess mode, lib is using the objects from ASP.NET Core integration. Which by the way are supported also in the isolated mode. https://learn.microsoft.com/en-us/azure/azure-functions/dotnet-isolated-process-guide?tabs=windows#http-trigger

HTTP triggers allow a function to be invoked by an HTTP request. There are two different approaches that can be used:

An ASP.NET Core integration model that uses concepts familiar to ASP.NET Core developers
A built-in model, which doesn't require extra dependencies and uses custom types for HTTP requests and responses. This approach is maintained for backward compatibility with previous .NET isolated worker apps.

Seems that ASP.NET Core integration model is the way to go forward if no compatibility issues are present. Should there be 2 versions of this extension, to deal with both?

@MarioMajcicaAtABNAMRO To me it makes sense to use HttpResponseData as that is part of the Microsoft.Azure.Functions.Worker.Http namespace, which you are most likely going to be using if you're using DurableTaskClientExtensions from the Microsoft.Azure.Functions.Worker namespace. Though admittedly I just followed suit with the class as it existed before my changes; there are other functions returning HttpResponseData in this class prior to my changes.

@MarioMajcicaAtABNAMRO To me it makes sense to use HttpResponseData as that is part of the Microsoft.Azure.Functions.Worker.Http namespace, which you are most likely going to be using if you're using DurableTaskClientExtensions from the Microsoft.Azure.Functions.Worker namespace. Though admittedly I just followed suit with the class as it existed before my changes; there are other functions returning HttpResponseData in this class prior to my changes.

The fact is that the built in model seems there for legacy reasons:

https://learn.microsoft.com/en-us/azure/azure-functions/dotnet-isolated-process-guide?tabs=windows#http-trigger

I'm okay with depending on HttpRequestData and HttpResponseData for now as part of this PR. I think there needs to be a separate effort for us to align with the ASP.NET Core integration model, which might be a larger and more complex work item.

Hey @dixonte thanks for your contributions and for opening this PR! To help get this merged, would you mind if I make some direct changes here? I’d like to add unit tests and merge the latest updates from our repo. Unfortunately, since this is a forked repo, I don’t have access to create a new branch, so making updates here would be the most efficient way. Thanks again for your understanding!

@nytian Absolutely, please do. All I want is for these changes to make it to an official build, so go for it.

@dixonte, I removed this section of code from this commit because it wasn’t functioning as expected. I’m not an expert on HTTP forwarding, so perhaps @jviau , who left the comments, might have additional context on this.

Anyways thanks again for this contribution. I think opening another PR might be better, as this change is somewhat separate from adding the WaitForCompletionOrCreateCheckStatusResponseAsync API, and separating it could also make the PR merge faster. If you're interested, we could address it there :)

@dixonte, I removed this section of code from this commit because it wasn’t functioning as expected. I’m not an expert on HTTP forwarding, so perhaps @jviau , who left the comments, might have additional context on this.

I think I had some issues getting the returned URIs to work when deployed to a linux host in Azure without those changes, that's why I bundled them in this PR. E.g. the status URI would have http://localhost:XXXXX at the start instead of the expected https://YYYYY.azurewebsites.net, somewhat defeating the entire purpose.

What do you mean, it wasn't functioning as expected, @nytian?